Skip to content

Don't sort on groupby (e.g. in skill() method)#376

Merged
jsmariegaard merged 29 commits intomainfrom
groupby-sort-false
Jan 8, 2024
Merged

Don't sort on groupby (e.g. in skill() method)#376
jsmariegaard merged 29 commits intomainfrom
groupby-sort-false

Conversation

@jsmariegaard
Copy link
Copy Markdown
Member

@jsmariegaard jsmariegaard commented Jan 4, 2024 •
edited
Loading

  • groupby(..., sort=False) in skill() and similar methods
    • correct tests accordingly
  • Fix bug in weights when assigning from dict (caused wrong results in some cases)
  • Re-introduce to_dataframe() on Comparer
  • Syncronize to_dataframe() of Comparer and Timeseries such that they work in the same way (include aux also)
  • Docstrings for all to_dataframe() methods
  • Docstrings for several other methods
  • Renaming of internal variables (and variables in tests) to confirm to best practice and for consistency (e.g. sk for skill() )
  • New method sort_values() on SkillTable
  • xy set to nan for skill() of track data

@jsmariegaard jsmariegaard changed the title Don't sort on groupby (e.g. from skill() method) Don't sort on groupby (e.g. in skill() method) Jan 5, 2024
@jsmariegaard jsmariegaard marked this pull request as ready for review January 5, 2024 10:04
@jsmariegaard
Copy link
Copy Markdown
Member Author

jsmariegaard commented Jan 7, 2024 •
edited
Loading

Now added more tests. Okay to merge @ecomodeller ?

@ecomodeller
Copy link
Copy Markdown
Member

ecomodeller commented Jan 8, 2024

Now added more tests. Okay to merge @ecomodeller ?

Good, and I changed the _to_long_dataframe a bit more.

@jsmariegaard jsmariegaard merged commit 64e17dd into main Jan 8, 2024
@jsmariegaard jsmariegaard deleted the groupby-sort-false branch January 8, 2024 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ecomodeller ecomodeller ecomodeller approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jsmariegaard @ecomodeller